-
Notifications
You must be signed in to change notification settings - Fork 297
added disable error log support #360
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
modified old PR owasp-modsecurity#327 Signed-off-by: Fatih USTA <[email protected]>
Hi @fatihusta, many thanks for update the mentioned patch. Could you add a new test case to the CI workflow, like this one? Just turn on this new feature, send an attack which triggers a rule, and check that the error.log is empty. Thanks! |
Thank you for this. I'm already using it :) My only feedback would be that the name of the variable is a bit confusing, you turn it ON to turn something OFF. Just my two cents |
Signed-off-by: Fatih USTA <[email protected]>
@fatihusta thanks for adding the test. What do you think about @tomsommer's idea. I think you should consider it - I agree with him, the current implementation has a bit weird logic. Also, after we agreed what should be the final keyword, please add the documentation into our README (README is part of the repository). |
- tests are changed with new directive name - nginx.conf updated with new directive name - added doc Signed-off-by: Fatih USTA <[email protected]>
|
Hi Thanks @tomsommer @airween |
Thanks - I'm going to check this soon. Until then, could you add this new keyword into README.md? |
I already added modsecurity_use_error_log key into README.md. |
modified old PR
#327